Update add missing param type from calls sniff to add missing doc blocks#2874
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThe PR updates a PHP_CodeSniffer sniff to detect missing function docblocks and create one (including ordered Changes
Sequence Diagram(s)(omitted — changes do not introduce multi-component sequential flows requiring visualization) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/phpunit/fields/test_FrmFieldGridHelper.php.php (1)
63-74: Fix PHPDoc type order to resolve pipeline failure.The pipeline is failing due to the
phpdoc_types_orderrule. The compound typeint|falselikely needs to havefalseafterintalphabetically, or the rule may expect a different ordering.Proposed fix
/** - * `@param` int|false $assert_size + * `@param` false|int $assert_size * `@param` string $assert_message */ private function sync_current_field_once( $assert_size = false, $assert_message = '' ) {Alternatively, run
vendor/bin/php-cs-fixer fixto auto-fix the ordering per project conventions.
🤖 Fix all issues with AI agents
In
`@phpcs-sniffs/Formidable/Sniffs/Commenting/AddMissingParamTypeFromCallsSniff.php`:
- Around line 578-595: The generated docblock is over-indented because the
builder appends an extra "\t" before each "*" line and before the function
declaration; update the $docblock construction in
AddMissingParamTypeFromCallsSniff (around $paramsToAdd processing) to remove
those extra tabs—use $indent . ' * `@param` ...' instead of $indent . "\t * `@param`
..." and change the footer concatenation from $indent . "\t */\n" . $indent .
"\t" to $indent . " */\n" . $indent so the block lines align with the function
line.
- Around line 78-86: Update the backward-scan ignore lists in both
findDocblock() and addDocblock() so PHP 8 attributes are skipped during docblock
detection/insertion: include T_ATTRIBUTE and T_ATTRIBUTE_END alongside the
existing ignored tokens used when scanning backwards from $stackPtr (in
findDocblock()) and when computing the insertion position (in addDocblock()),
ensuring the scan will pass over attribute blocks and prevent inserting a
duplicate docblock below attributes.
In `@tests/phpunit/emails/test_FrmEmail.php`:
- Around line 753-757: The docblock for the private method
check_private_properties is missing the `@param` annotation for the first argument
$settings; update the docblock to include "@param array $settings" (or the
appropriate type if different) so it documents the method signature correctly
alongside the existing `@param` entries for $setting_name and $property.
In `@tests/phpunit/forms/test_FrmForm.php`:
- Around line 183-185: The docblock above the function is missing documentation
for the by-reference parameter $value; update the PHPDoc for the function in
test_FrmForm.php (look for the docblock containing "@param string $opt") to add
an additional `@param` entry for $value that includes the by-ref marker (&), an
appropriate type (e.g. mixed or string depending on usage) and a short
description (for example: "@param mixed &$value — value passed by reference to
be modified"). Ensure the new `@param` appears alongside the existing "@param
string $opt" entry.
In `@tests/phpunit/forms/test_FrmFormsController.php`:
- Around line 155-157: The docblock currently documents only $post_content; add
a `@param` entry for $form_id (e.g., "@param int $form_id" or the correct type
used in the function) to the same docblock so both parameters ($form_id and
$post_content) are documented and include a short description for $form_id;
update the docblock immediately above the helper/function that accepts $form_id
and $post_content (referencing the $form_id and $post_content symbols).
| // Check if function has a docblock. | ||
| $docblock = $this->findDocblock( $phpcsFile, $stackPtr ); | ||
| $hasDocblock = ( false !== $docblock ); | ||
| $existingParams = array(); | ||
|
|
||
| if ( $hasDocblock ) { | ||
| // Get existing @param tags with their types. | ||
| $existingParams = $this->getExistingParamTags( $phpcsFile, $docblock ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files | grep -i "addmissingparamtypefromcallssniff"Repository: Strategy11/formidable-forms
Length of output: 150
🏁 Script executed:
# Check if the file exists and get its line count
wc -l phpcs-sniffs/Formidable/Sniffs/Commenting/AddMissingParamTypeFromCallsSniff.phpRepository: Strategy11/formidable-forms
Length of output: 154
🏁 Script executed:
# Read the findDocblock method to understand its current implementation
sed -n '1,600p' phpcs-sniffs/Formidable/Sniffs/Commenting/AddMissingParamTypeFromCallsSniff.php | cat -nRepository: Strategy11/formidable-forms
Length of output: 19995
🏁 Script executed:
# Search for any existing references to attributes in the codebase
rg -i "attribute" --type php | head -20Repository: Strategy11/formidable-forms
Length of output: 2255
🌐 Web query:
PHP CodeSniffer T_ATTRIBUTE token PHP 8 attributes
💡 Result:
In PHP_CodeSniffer (PHPCS), PHP 8 attributes are represented using the tokenizer’s T_ATTRIBUTE token (marks the start of an attribute). PHPCS also defines an additional token T_ATTRIBUTE_END (marks the end of an attribute), and adds attribute_owner / attribute_closer indexes to tokens inside the attribute to help sniffs relate the attribute to the thing it decorates. This support was added in PHPCS 3.6.0 (released 2021-04-08), and PHPCS also “backfills” attribute tokenization when running on older PHP versions. [1] [2] [3]
Practical sniff notes:
- To detect attributes, register for
T_ATTRIBUTE(and often also useT_ATTRIBUTE_ENDto skip/scan cleanly). [1] [2] - In PHPCS code, these are exposed as PHPCS token constants (e.g.
\PHP_CodeSniffer\Util\Tokens::T_ATTRIBUTE,::T_ATTRIBUTE_END). [2]
Sources: [1] [2] [3]
🏁 Script executed:
# Check if there are any test cases or examples with PHP 8 attributes in the repo
find . -type f -name "*.php" -exec grep -l "#\[" {} \;Repository: Strategy11/formidable-forms
Length of output: 143
🏁 Script executed:
# Check the files that use PHP 8 attributes to understand the pattern
head -50 classes/models/FrmSettings.php | grep -A 5 "#\["Repository: Strategy11/formidable-forms
Length of output: 144
🏁 Script executed:
# Also check the addDocblock method more carefully for the same issue
sed -n '541,599p' phpcs-sniffs/Formidable/Sniffs/Commenting/AddMissingParamTypeFromCallsSniff.php | cat -nRepository: Strategy11/formidable-forms
Length of output: 2148
🏁 Script executed:
# Check if there are any other places in the sniff that scan backward looking for tokens
rg "findPrevious" phpcs-sniffs/Formidable/Sniffs/Commenting/AddMissingParamTypeFromCallsSniff.php -B 2 -A 2Repository: Strategy11/formidable-forms
Length of output: 1013
🏁 Script executed:
# Let's verify the PHPCS version or constants used in the project
rg -i "require.*phpcs" phpcs-sniffs/ composer.json composer.lock -A 2 | head -30Repository: Strategy11/formidable-forms
Length of output: 949
🏁 Script executed:
# Check if T_ATTRIBUTE is referenced anywhere in the codebase already
rg "T_ATTRIBUTE" --type phpRepository: Strategy11/formidable-forms
Length of output: 53
🏁 Script executed:
# Look at the specific logic in findDocblock more carefully with context
sed -n '188,208p' phpcs-sniffs/Formidable/Sniffs/Commenting/AddMissingParamTypeFromCallsSniff.php | cat -nRepository: Strategy11/formidable-forms
Length of output: 671
Add PHP 8 attribute token handling in both findDocblock() and addDocblock() methods.
When a docblock sits above PHP 8 attributes (e.g., #[SomeAttribute]), findDocblock() will currently miss it because the backward scan stops at the attribute token, causing a duplicate docblock to be inserted below the attributes. The same issue exists in addDocblock() which determines insertion position using an identical pattern. Add T_ATTRIBUTE and T_ATTRIBUTE_END to the ignore lists in both methods:
🔧 Required changes in two locations
Location 1: findDocblock() method (line 191-199)
$ignore = array(
T_WHITESPACE,
+ T_ATTRIBUTE,
+ T_ATTRIBUTE_END,
T_STATIC,
T_PUBLIC,
T_PRIVATE,
T_PROTECTED,
T_ABSTRACT,
T_FINAL,
);Location 2: addDocblock() method (line 548)
$prev = $phpcsFile->findPrevious(
- array( T_WHITESPACE, T_STATIC, T_PUBLIC, T_PRIVATE, T_PROTECTED, T_ABSTRACT, T_FINAL ),
+ array( T_WHITESPACE, T_ATTRIBUTE, T_ATTRIBUTE_END, T_STATIC, T_PUBLIC, T_PRIVATE, T_PROTECTED, T_ABSTRACT, T_FINAL ),
$stackPtr - 1,
null,
true
);🤖 Prompt for AI Agents
In
`@phpcs-sniffs/Formidable/Sniffs/Commenting/AddMissingParamTypeFromCallsSniff.php`
around lines 78 - 86, Update the backward-scan ignore lists in both
findDocblock() and addDocblock() so PHP 8 attributes are skipped during docblock
detection/insertion: include T_ATTRIBUTE and T_ATTRIBUTE_END alongside the
existing ignored tokens used when scanning backwards from $stackPtr (in
findDocblock()) and when computing the insertion position (in addDocblock()),
ensuring the scan will pass over attribute blocks and prevent inserting a
duplicate docblock below attributes.
| // Build the docblock. | ||
| $docblock = "/**\n"; | ||
|
|
||
| // Sort params by index to maintain order. | ||
| uasort( | ||
| $paramsToAdd, | ||
| function ( $a, $b ) { | ||
| return $a['index'] - $b['index']; | ||
| } | ||
| ); | ||
|
|
||
| foreach ( $paramsToAdd as $paramName => $info ) { | ||
| $typeString = implode( '|', $info['types'] ); | ||
| $docblock .= $indent . "\t * @param " . $typeString . ' ' . $paramName . "\n"; | ||
| } | ||
|
|
||
| $docblock .= $indent . "\t */\n" . $indent . "\t"; | ||
|
|
There was a problem hiding this comment.
Fix indentation in generated docblocks.
The current builder adds an extra tab for each * line and before the function, which will over-indent the block and the declaration.
🛠️ Adjust indentation to match the function line
- foreach ( $paramsToAdd as $paramName => $info ) {
- $typeString = implode( '|', $info['types'] );
- $docblock .= $indent . "\t * `@param` " . $typeString . ' ' . $paramName . "\n";
- }
+ foreach ( $paramsToAdd as $paramName => $info ) {
+ $typeString = implode( '|', $info['types'] );
+ $docblock .= $indent . " * `@param` " . $typeString . ' ' . $paramName . "\n";
+ }
- $docblock .= $indent . "\t */\n" . $indent . "\t";
+ $docblock .= $indent . " */\n" . $indent;🤖 Prompt for AI Agents
In
`@phpcs-sniffs/Formidable/Sniffs/Commenting/AddMissingParamTypeFromCallsSniff.php`
around lines 578 - 595, The generated docblock is over-indented because the
builder appends an extra "\t" before each "*" line and before the function
declaration; update the $docblock construction in
AddMissingParamTypeFromCallsSniff (around $paramsToAdd processing) to remove
those extra tabs—use $indent . ' * `@param` ...' instead of $indent . "\t * `@param`
..." and change the footer concatenation from $indent . "\t */\n" . $indent .
"\t" to $indent . " */\n" . $indent so the block lines align with the function
line.
| /** | ||
| * @param string $setting_name | ||
| * @param string $property | ||
| */ | ||
| private function check_private_properties( $settings, $setting_name, $property = '' ) { |
There was a problem hiding this comment.
Missing @param for $settings parameter.
The docblock is missing the first parameter $settings. Based on usage throughout the file, it should be an array.
Proposed fix
/**
+ * `@param` array $settings
* `@param` string $setting_name
* `@param` string $property
*/
private function check_private_properties( $settings, $setting_name, $property = '' ) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * @param string $setting_name | |
| * @param string $property | |
| */ | |
| private function check_private_properties( $settings, $setting_name, $property = '' ) { | |
| /** | |
| * `@param` array $settings | |
| * `@param` string $setting_name | |
| * `@param` string $property | |
| */ | |
| private function check_private_properties( $settings, $setting_name, $property = '' ) { |
🤖 Prompt for AI Agents
In `@tests/phpunit/emails/test_FrmEmail.php` around lines 753 - 757, The docblock
for the private method check_private_properties is missing the `@param` annotation
for the first argument $settings; update the docblock to include "@param array
$settings" (or the appropriate type if different) so it documents the method
signature correctly alongside the existing `@param` entries for $setting_name and
$property.
| /** | ||
| * @param string $opt | ||
| */ |
There was a problem hiding this comment.
Add missing @param for $value (by-ref).
$value is still undocumented, which undercuts the goal of these docblock additions.
💡 Suggested docblock update
/**
* `@param` string $opt
+ * `@param` string &$value
*/
private function sanitize_field_opt( $opt, &$value ) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * @param string $opt | |
| */ | |
| /** | |
| * `@param` string $opt | |
| * `@param` string &$value | |
| */ |
🤖 Prompt for AI Agents
In `@tests/phpunit/forms/test_FrmForm.php` around lines 183 - 185, The docblock
above the function is missing documentation for the by-reference parameter
$value; update the PHPDoc for the function in test_FrmForm.php (look for the
docblock containing "@param string $opt") to add an additional `@param` entry for
$value that includes the by-ref marker (&), an appropriate type (e.g. mixed or
string depending on usage) and a short description (for example: "@param mixed
&$value — value passed by reference to be modified"). Ensure the new `@param`
appears alongside the existing "@param string $opt" entry.
| /** | ||
| * @param array $post_content | ||
| */ |
There was a problem hiding this comment.
Document $form_id in the helper docblock.
The new docblock only documents $post_content, so $form_id remains undocumented.
💡 Suggested docblock update
/**
+ * `@param` int $form_id
* `@param` array $post_content
*/
private function create_on_submit_action( $form_id, $post_content ) {🤖 Prompt for AI Agents
In `@tests/phpunit/forms/test_FrmFormsController.php` around lines 155 - 157, The
docblock currently documents only $post_content; add a `@param` entry for $form_id
(e.g., "@param int $form_id" or the correct type used in the function) to the
same docblock so both parameters ($form_id and $post_content) are documented and
include a short description for $form_id; update the docblock immediately above
the helper/function that accepts $form_id and $post_content (referencing the
$form_id and $post_content symbols).
…e_from_calls_sniff_to_add_missing_doc_blocks Update add missing param type from calls sniff to add missing doc blocks
1 participant
Summary by CodeRabbit
Improvements
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.